Skip to content

Conversation

@lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Jul 17, 2025

In the JSON editor ie. the middle one of the three in the Documents tab. Suppose you have this document:

{
  a: Double(1)
  b: Long(1)
  object: {
    c: Double(1)
    d: Long(1)
  }
}

and you edit it in the JSON editor, it gets serialised as

{
  "a": 1,
  "b": 1,
  "object": {
    "c": 1,
    "d": 1
  }
}

so when you save that back it becomes

{
  a: Int32(1)
  b: Int32(1)
  object: {
    c: Int32(1)
    d: Int32(1)
  }
}

ie. we implicitly change the type of the Double or Long into an Int32 because 1 can safely be represented by a 32bit int.

This change will check the type of a property and if it is now Int32 and it was Double or Long, it will turn it back into a Double or Long when you save. So the document keeps its original "unnecessary" type.

Just to be clear: This only applies to the json editor. The other editors are unaffected.


In case you're wondering why this is broken in the json editor but not in the document editor:

The reason why we lose the type information is because it isn't encoded in the JSON and then we do HadronDocument.FromEJSON(value || '') which just re-infers all the types using TypeChecker.type(). TypeChecker.type() will use _bsontype if it is set, otherwise it does this thing where it tries to figure out if it should be a double or an int.

Because it uses _bsontype if set and because the double and Int64 editors explicitly cast the type, the exact type is never lost in the document editor.


I decided to recurse on objects, but leave arrays alone. It is really tricky and error-prone to detect if the user's intention was to have the whole array use Long and Double and never Int32 or not. Although it would have potentially been useful for embeddings / vectors where the values just might end up as a round number in an array that is otherwise doubles. I tried a few things but gave up on that idea every time because it just felt inherently dangerous. We could re-visit the idea in the future if necessary. Follow-up ticket: https://jira.mongodb.org/browse/COMPASS-9578

Copilot AI review requested due to automatic review settings July 17, 2025 11:15
@lerouxb lerouxb requested a review from a team as a code owner July 17, 2025 11:15
@github-actions github-actions bot added the fix label Jul 17, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures that Double and Long types are preserved when editing numeric values in the JSON editor by comparing against the original document and restoring types where needed.

  • Added preserveType on Element and preserveTypes on Document to recursively revert Int32 back to Double or Int64 when the original type was different.
  • Integrated preserveTypes into the JSON editor’s onUpdate flow before applying changes.
  • Introduced tests covering Double and Long preservation scenarios.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/hadron-document/test/document.test.ts Added tests for preserveTypes on Document
packages/hadron-document/src/element.ts Implemented preserveType for individual Element objects
packages/hadron-document/src/document.ts Implemented preserveTypes for top‐level Document objects
packages/compass-crud/src/components/json-editor.tsx Applied preserveTypes in the JSON editor update handler
Comments suppressed due to low confidence (3)

packages/hadron-document/test/document.test.ts:2396

  • Add tests to verify that array elements retain their types (or are intentionally ignored) after calling preserveTypes, since arrays are specifically skipped.
    describe('#preserveTypes', function () {

packages/hadron-document/src/element.ts:203

  • [nitpick] Add a JSDoc comment explaining what preserveType does, its recursion over objects, and why arrays are intentionally not handled.
  preserveType(otherElement: Element): void {

packages/hadron-document/src/document.ts:111

  • [nitpick] Add a JSDoc comment for preserveTypes, describing its purpose, high-level algorithm, and decision to leave arrays unchanged.
  preserveTypes(other: Document): void {

@lerouxb lerouxb force-pushed the json-preserve-type branch from 413eaf5 to b360ea4 Compare July 17, 2025 11:22
@lerouxb lerouxb merged commit 4d5f333 into main Jul 17, 2025
50 of 51 checks passed
@lerouxb lerouxb deleted the json-preserve-type branch July 17, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants